refactor(amber): remove dead WorkflowWorker.handleActorCommand#6305
Conversation
Automated Reviewer SuggestionsBased on the
|
There was a problem hiding this comment.
Pull request overview
This PR removes an unreachable debug-only handleActorCommand: Receive method from WorkflowWorker in Amber’s worker actor implementation, reducing dead code and avoiding misleading/unused code paths.
Changes:
- Deleted the unused
WorkflowWorker.handleActorCommandreceive handler that only printedActorCommands. - Keeps the actor’s actual
receivechain unchanged (super.receive orElse handleDirectInvocation orElse handleTriggerClosure).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Xinyuan Lin <xinyual3@uci.edu>
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 548 | 0.334 | 17,354/23,767/23,767 us | 🔴 -10.2% / 🔴 +57.2% |
| 🟢 | bs=100 sw=10 sl=64 | 1,171 | 0.715 | 84,147/97,360/97,360 us | 🟢 -11.0% / 🟢 +17.1% |
| ⚪ | bs=1000 sw=10 sl=64 | 1,361 | 0.83 | 733,601/783,914/783,914 us | ⚪ within ±5% / 🟢 +33.5% |
Baseline details
Latest main 12f9f1a from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 548 tuples/sec | 609 tuples/sec | 779.42 tuples/sec | -10.0% | -29.7% |
| bs=10 sw=10 sl=64 | MB/s | 0.334 MB/s | 0.372 MB/s | 0.476 MB/s | -10.2% | -29.8% |
| bs=10 sw=10 sl=64 | p50 | 17,354 us | 16,199 us | 12,550 us | +7.1% | +38.3% |
| bs=10 sw=10 sl=64 | p95 | 23,767 us | 25,266 us | 15,115 us | -5.9% | +57.2% |
| bs=10 sw=10 sl=64 | p99 | 23,767 us | 25,266 us | 18,652 us | -5.9% | +27.4% |
| bs=100 sw=10 sl=64 | throughput | 1,171 tuples/sec | 1,173 tuples/sec | 1,000 tuples/sec | -0.2% | +17.1% |
| bs=100 sw=10 sl=64 | MB/s | 0.715 MB/s | 0.716 MB/s | 0.61 MB/s | -0.1% | +17.1% |
| bs=100 sw=10 sl=64 | p50 | 84,147 us | 82,066 us | 99,872 us | +2.5% | -15.7% |
| bs=100 sw=10 sl=64 | p95 | 97,360 us | 109,354 us | 107,073 us | -11.0% | -9.1% |
| bs=100 sw=10 sl=64 | p99 | 97,360 us | 109,354 us | 114,377 us | -11.0% | -14.9% |
| bs=1000 sw=10 sl=64 | throughput | 1,361 tuples/sec | 1,354 tuples/sec | 1,020 tuples/sec | +0.5% | +33.5% |
| bs=1000 sw=10 sl=64 | MB/s | 0.83 MB/s | 0.826 MB/s | 0.622 MB/s | +0.5% | +33.3% |
| bs=1000 sw=10 sl=64 | p50 | 733,601 us | 737,344 us | 987,499 us | -0.5% | -25.7% |
| bs=1000 sw=10 sl=64 | p95 | 783,914 us | 756,487 us | 1,028,784 us | +3.6% | -23.8% |
| bs=1000 sw=10 sl=64 | p99 | 783,914 us | 756,487 us | 1,057,136 us | +3.6% | -25.8% |
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,365.25,200,128000,548,0.334,17354.01,23767.28,23767.28
1,100,10,64,20,1707.99,2000,1280000,1171,0.715,84146.84,97360.01,97360.01
2,1000,10,64,20,14699.45,20000,12800000,1361,0.830,733600.82,783914.21,783914.21
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #6305 +/- ##
============================================
- Coverage 60.77% 60.75% -0.02%
+ Complexity 3367 3363 -4
============================================
Files 1135 1135
Lines 44191 44188 -3
Branches 4830 4829 -1
============================================
- Hits 26855 26848 -7
Misses 15878 15878
- Partials 1458 1462 +4
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
What changes were proposed in this PR?
Removes the dead
handleActorCommandmethod fromWorkflowWorker. It is aReceivewhose body is a bareprintln(c), it is not part of the actor'sreceivechain (receive = super.receive orElse handleDirectInvocation orElse handleTriggerClosure), and it has no callers — the livehandleActorCommandis a separate method onDPThread. Deleting it removes a debug leftover and raises coverage by removal rather than cementing dead code with a test.Any related issues, documentation, discussions?
Closes #6304. Follow-up to a review comment by @mengw15 on #6289.
How was this PR tested?
No new tests — this only deletes unreachable code. Verified locally (Java 17): the amber module compiles,
WorkerSpec(which exercisesWorkflowWorker) passes, andscalafixAll --checkis clean (theActorCommandimport remains used viaActorCommandElement).Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8 [1M context])